fix(vdd): fallback display modes after CCD failures#713
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough在 Windows 显示设备设置路径中为 VDD(ZAKO_NAME)场景添加专用应用策略:直接应用失败时先按延迟重试(250/500/1000ms),仍失败则生成并按序尝试回退候选(降频至120/60或固定1920x1080@60),首次成功即返回。 ChangesVDD 显示模式重试与回退流程
评估指标🎯 4 (Complex) | ⏱️ ~45 minutes 可能关联的 Issue
可能关联的 PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/windows/display_device/settings.cpp`:
- Around line 220-255: The current implementation of with_display_mode and
make_vdd_fallback_modes abuses a single display_mode_t from requested_modes
(std::begin(requested_modes)->second) and then overwrites every entry in
device_display_mode_map_t with that one mode, which incorrectly forces all
devices to the same fallback; instead either (A) change make_vdd_fallback_modes
to produce a device-specific device_display_mode_map_t (i.e., build per-device
candidates from each entry in requested_modes) and stop using a single prototype
mode, or (B) if you intend to keep a single-mode path, assert early in
make_vdd_fallback_modes that all entries in requested_modes are identical (and
fail/return early otherwise) before calling with_display_mode; update callers
(e.g., any set_display_modes usage noted around the other affected block) to
handle per-device fallbacks accordingly. Ensure references: with_display_mode,
make_vdd_fallback_modes, device_display_mode_map_t, requested_modes,
set_display_modes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b80822e7-ccaf-4495-ae69-5cbf9efdfc7e
📒 Files selected for processing (1)
src/platform/windows/display_device/settings.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/platform/windows/display_device/settings.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/settings.cpp
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/windows/display_device/settings.cpp`:
- Around line 255-277: The current loops that generate fallback candidates skip
modes like 119.88Hz and 59.94Hz because they use strict >121.0 / >61.0 checks;
update the checks in the candidate generation loops (the loops iterating over
candidate_modes and using refresh_rate_hz(mode.refresh_rate)) to include rates
that are close to the integer targets—e.g. treat a mode as a 120Hz candidate
when refresh_rate_hz(...) >= 119.5 (or fabs(refresh_rate_hz(...) - 120.0) <=
0.5) and as a 60Hz candidate when refresh_rate_hz(...) >= 59.5 (or fabs(... -
60.0) <= 0.5), then set mode.refresh_rate = refresh_rate_t { 120, 1 } / { 60, 1
} and call add_candidate(candidate_modes) exactly as before (symbols:
candidate_modes, requested_modes, refresh_rate_hz, refresh_rate_t,
add_candidate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed95c6cf-3765-4544-94c5-10cba7e60d53
📒 Files selected for processing (1)
src/platform/windows/display_device/settings.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/platform/windows/display_device/settings.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/settings.cpp
🔇 Additional comments (1)
src/platform/windows/display_device/settings.cpp (1)
219-252: LGTM!Also applies to: 313-319
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/display_device/session.cpp`:
- Around line 504-507: The early return using vdd_settings.needs_update
incorrectly treats that flag as "session mode already active" — replace this
short‑circuit so the XML+reload fallback can run when set_vdd_session_mode()
returns interface_missing; specifically, remove or relax the if
(!vdd_settings.needs_update) { return; } check in session.cpp and instead gate
skipping on a real "confirmed current mode" state (or always attempt the
XML+reload path when set_vdd_session_mode() reports interface_missing); ensure
logic around prepare_vdd_settings and the call site of set_vdd_session_mode() is
updated so needs_update only reflects cache status and not runtime mode
confirmation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e40c5467-701b-46a5-9773-43da0ddf1106
📒 Files selected for processing (1)
src/display_device/session.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/display_device/session.cpp
🔇 Additional comments (1)
src/display_device/session.cpp (1)
587-590: LGTM!
改了啥呀
set_display_modes()三段式逻辑;只有目标全是Zako HDR且失败时,才进入补强路径。120Hz、同分辨率60Hz,最后1920x1080@60。SETMODES同步本次请求的运行态 mode;标准分辨率也走这条路径,不再只有“配置列表里没有的模式”才同步。为啥要改
BlackCat 的日志显示 VDD 已经创建出来,但
2560x1600@144在SetDisplayConfig的 recommended / strict / explicit target 三种方式里都被 Windows 返回ERROR_GEN_FAILURE/1610拒绝。后续用户日志里又看到更明确的一类场景:
1920x1080@60已经在配置列表里,但 VDD 创建后直接让 CCD 应用该模式仍失败;随后客户端请求2296x1080@60时先触发SETMODES,同一个 VDD 设备就能成功应用并进入串流。这说明问题不只是“非标准分辨率不稳”,也可能是 VDD 运行态 mode list 和 Sunshine 后续 CCD 请求没有先同步。继续直接 503 会让用户感觉“标准分辨率都坏了”,但实际更像 VDD mode 状态没准备好。
这次改成:会话开始先把本次 mode 推给 VDD,再让 Windows 应用显示模式;失败后再按 VDD-only fallback 链路降级。能自己兜就自己兜,别把杂鱼 503 直接甩给用户。
验证
git diff --check -- src/display_device/session.cpp src/platform/windows/display_device/settings.cppsunshine-logs-2026-06-07T13-41-39.txt:1920x1080@60两次未触发SETMODES且 CCD 失败;2296x1080@60触发SETMODES后成功应用并开始串流。cmake --preset dev-win -DPKG_CONFIG_EXECUTABLE=C:/Strawberry/perl/bin/pkg-config.bat未完成:当前本地工具链缺libcurl.pc,配置停在依赖发现阶段,还没进入本次 C++ 改动编译。CI 需要继续验证 Windows build。